Fix firedancer-dev bench startup crash#8565
Fix firedancer-dev bench startup crash#8565ripatel-fd wants to merge 1 commit intofiredancer-io:mainfrom
Conversation
ripatel-fd
commented
Feb 27, 2026
ripatel-fd
commented
- Enable sandbox when watch is disabled
- Add support for 'firedancer-dev mem --topo bench'
- Fix segfault on startup
There was a problem hiding this comment.
Pull request overview
Updates the shared dev bench command wiring so topology construction is available independently of execution (for mem --topo bench), adds a --no-watch option, and adjusts startup behavior to avoid early crashes/segfaults.
Changes:
- Split bench topology construction into a new
bench_topo(config_t *)and register it as the action.topohook. - Add
--no-watchCLI support (newargs->load.no_watch) and conditionally disable sandbox only when watch mode is enabled. - Rework
--no-quichandling by post-processingbenchstiles inbench_cmd_fn().
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/app/shared_dev/commands/bench/bench.h | Exposes bench_topo() and updates bench_cmd_fn() signature. |
| src/app/shared_dev/commands/bench/bench.c | Implements bench_topo(), adds --no-watch parsing, and changes --no-quic handling. |
| src/app/shared/fd_action.h | Adds no_watch to the shared load args (also used by bench). |
| src/app/firedancer-dev/commands/bench.c | Registers bench_topo and updates call to new bench_cmd_fn() signature. |
| src/app/fddev/commands/bench.c | Registers bench_topo and updates call to new bench_cmd_fn() signature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ripatel-fd
force-pushed
the
bench-crash
branch
from
5b249d8 to
e85913e
Compare
- Enable sandbox when watch is disabled - Add support for 'firedancer-dev mem --topo bench' - Fix segfault on startup - Work around invalid RPC tile responses on startup (status 500)
ripatel-fd
force-pushed
the
bench-crash
branch
from
e85913e to
20ba16e
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| void | ||
| bench_cmd_fn( args_t * args, | ||
| config_t * config, | ||
| int watch ) { | ||
| fd_topo_initialize( config_t * config ); | ||
|
|
||
| ushort dest_port = fd_ushort_if( args->load.no_quic, | ||
| config->tiles.quic.regular_transaction_listen_port, | ||
| config->tiles.quic.quic_transaction_listen_port ); | ||
| void | ||
| bench_topo( config_t * config ) { | ||
| fd_topo_initialize( config ); |
There was a problem hiding this comment.
The fd_ushort_if call in the new bench_topo function has redundant arguments - both the condition c and the true-branch value t are config->frankendancer.rpc.port, making the expression equivalent to config->frankendancer.rpc.port ? config->frankendancer.rpc.port : 8899. This could be written more clearly as a simple conditional assignment: if( !config->frankendancer.rpc.port ) config->frankendancer.rpc.port = 8899;. The intent is to set a default port of 8899 when none is configured.
